Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add String support for GDScript Enums #21014

Closed

Conversation

willnationsdev
Copy link
Contributor

Fixes #20988.

This PR also makes it possible to create enum value assignment using a colon (Dictionary syntax) in addition to the equals sign (original enum syntax). Since these effectively create Dictionaries anyway, it makes sense to allow either syntax for this purpose rather than arbitrarily erroring on the Dictionary syntax.

@willnationsdev
Copy link
Contributor Author

willnationsdev commented Aug 14, 2018

Just tested this on my machine with both "=" and ":". Seems like it's all working, and I just fixed all of the syntax check errors (some weren't even made by me, so I'm not sure how they managed to get into my build).

Edit: realized I only tested declarations, but haven't tested actually exporting the values yet. I suspect that will require a bit more work.

Edit 2: Double checked exporting / changing the exported value. Everything there seems to work too.

@akien-mga
Copy link
Member

@vnen Could you have a look at this PR?

Will need a rebase eventually.

@akien-mga akien-mga modified the milestones: 3.1, 3.2 Dec 18, 2018
@akien-mga akien-mga removed the request for review from reduz April 30, 2019 15:21
@akien-mga
Copy link
Member

We discussed this on IRC with @vnen and @bojidar-bg, and agreed that this proposal and the related issue are likely not the right approach to solve a valid issue (getting a string from an (int) enum value). Logs:

15:07 <Akien> Next: #21014
15:07 <IssueBot> #21014: Add String support for GDScript Enums | https://git.io/fNNKC
15:07 <bojidar_bg> NB: I'm biased towards close here
15:07 <Akien> I'm not sure we should have this feature in the first place either.
15:08 <bojidar_bg> it might make enum-typed parameters harder to implement
15:09 <vnen> I don't think it's needed. I see it as a solution to a lack in the language somewhere else
15:10 <bojidar_bg> Also, it will break with export(Enum)
15:10 <vnen> for instance, it should be easier to get the name from an enum value
15:10 <bojidar_bg> (ah, nevermind my last message)
15:11 <Akien> Yeah, being able to get the enum identifier as a string would be quite nice.
15:11 <Akien> It seems like this PR only means to address the inconsistency between having enum String hints in exports, which map to int values
15:12 <vnen> I do: `MY_ENUM.keys()[enum_value]`, but that doesn't work with custom assignments
15:13 <bojidar_bg> Maybe something like Dictionary.reverse() which builds a dictionary by swapping the key and value in each entry?
15:14 <bojidar_bg> or, something like what I've done in the asset library: enums which have both the string key -> integer value mapping and the integer value -> string key; this would allow for doing `MY_ENUM[enum_value]` directly
15:15 <Akien> Note that it would also be useful to retrieve string names from built-in enums and constants, like converting Error codes to something user readable.
15:16 <bojidar_bg> Well, another option would be to have a Variant::Enum type which contains an integer and enum name/dictionary/?
15:17 <bojidar_bg> ... which would basically be a Symbol/StringName-like type with a custom lookup table..
15:18 <vnen> I guess we agree to close the PR, we can discuss a solution later
15:18 <bojidar_bg> we should probably close the issue as well, and find another one for the discussion

@bojidar-bg Can I let you open the follow-up issue?

@Razzlegames
Copy link
Contributor

Just curious if there has been any follow up issue created for this? @akien-mga @bojidar-bg ? Thanks

@Razzlegames
Copy link
Contributor

Nevermind. Found it and linking here for easier reference: #31896

Sauermann referenced this pull request in godotengine/godot-docs Apr 8, 2020
@willnationsdev willnationsdev deleted the string-enums branch May 16, 2020 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add String-based Enum support to GDScript
4 participants